Add support for unevaluatedProperties + a few other changes.
#419
Conversation
|
Seeing the CI: I'll rebase this locally in a little while to get this using a proper conventional commit message. |
tobz
force-pushed
the
draft-2019-09-unevaluated-properties
branch
from
1108842
to
e43f0fd
Compare
tobz
force-pushed
the
draft-2019-09-unevaluated-properties
branch
from
e43f0fd
to
a37e8ca
Compare
Codecov Report
@@ Coverage Diff @@
## master #419 +/- ##
==========================================
+ Coverage 78.59% 79.40% +0.80%
==========================================
Files 54 56 +2
Lines 4481 5015 +534
==========================================
+ Hits 3522 3982 +460
- Misses 959 1033 +74
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
Great PR and a great write-up! :) Thank you so much for this! |
|
@Stranger6667 🙇🏻 Happy to be able to contribute back. Do you have a typical release cycle for |
|
I don't but I'd like to make a new release today :) |
|
Great, I'll keep an eye out for when that happens. Appreciate the timeliness on the review and the release; certainly did not expect my PR to be getting this much traction so quickly. 😂 |
|
My pleasure :) Happy to see your contribution and that's great that |
|
|
Closes #288.
This PR adds support for
unevaluatedPropertieswhen using thedraft201909ordraft202012feature flags.Overall approach
I took a somewhat similar approach to the Python code linked in the issue for this feature, where the validator for
unevaluatedPropertiesuses a subvalidator for each relevant keyword --properties,patternProperties,dependentSchemas,$ref, etc -- that performs the relevant logic for determining if the given schema for the keyword would evaluate the property.Each subvalidator exposes an interface that looks a lot like the
Validatetrait, but is oriented towards checking solely if a property would be valid against the schema, and has an optional return value to indicate evaluated vs not evaluated. The validator code, overall, is a bit verbose, not unlikeadditionalProperties... but there's a possibility of turning the API into an actual trait which would theoretically allow us to reduce a lot of boilerplate when we check each subvalidator inUnevaluatedPropertiesValidator::is_valid_property, and so on.Code deduplication and refactoring
Beyond that, I ended up refactoring a little bit of the shared code between
unevaluatedPropertiesandadditionalProperties. I tried, initially, to carry over the logic for switching between a small and big validator map, but it got a little too complex when considering thatUnevaluatedPropertiesValidatoris inherently recursive, and it didn't make a lot of sense propagating a generic validator map type all the way through. I'm happy to try and revisit this if you think it matters enough.Fixing
$reffor draft 2019-09 and newerIn order to fully support
unevaluatedProperties, I had to do a small bit of refactoring in terms of how$refis handled. This also had the nice side effect of fixing$refusage when using draft 2019-09 and newer, as now it can be evaluated alongside other keywords.Beyond that, there's just a lot of boilerplate, and I tried to pick a code style/pattern and just stick to that and be consistent about it, but I'm happy to do any of the aforementioned refactoring ideas, or anything else, to bring the code more in line with your expectations.